New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace lodash 'clone' usage with ES6 Spread initializer #11811
Replace lodash 'clone' usage with ES6 Spread initializer #11811
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/25669/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 6da65ba:
|
@@ -106,7 +105,7 @@ export function explode(visitor) { | |||
if (existing) { | |||
mergePair(existing, fns); | |||
} else { | |||
visitor[alias] = clone(fns); | |||
visitor[alias] = { ...fns }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jayaddison Sorry I missed this while it was open but I think this could introduce an edge case behavior change if fns
has any non-enumerable values in the prototype chain. A better replacement might be:
visitor[alias] = Object.create(Object.getPrototypeOf(fns));
Object.assign(visitor[alias], fns);
Here the cloned object is created with the prototype of fns
, then Object.assign
copies only the own iterable properties from fns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem at all, thanks for catching this. Fixup opened in #11820.
Continues removal of lodash function dependencies identified per #11789.